Conversation
Performance Report (Linux) ✅
Legend
|
Test Coverage Report (Linux)
Coverage increased! Great work! |
Performance Report (macOS)
Legend
|
Performance Report (Windows) ➖
Legend
|
Test Coverage Report (Windows)
Coverage increased! Great work! |
There was a problem hiding this comment.
Pull request overview
This PR fixes a race between configure and refresh by ensuring configuration publication and long-lived shared locator reconfiguration occur under the same Context.configuration write lock, preventing refresh-related state sync from interleaving with locator reconfiguration.
Changes:
- Refactors
handle_configure()to apply config updates + shared locator reconfiguration atomically via a newapply_configure_options()helper. - Ensures shared locator reconfiguration happens while holding the configuration write lock to prevent readers observing partially-applied state.
- Adds a regression test that blocks inside a locator’s
configure()to verify configuration readers can’t observe the new generation/config early.
Show a summary per file
| File | Description |
|---|---|
crates/pet/src/jsonrpc.rs |
Serializes configure publication and shared locator reconfiguration under the configuration write lock; adds regression coverage for the race. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 2
There was a problem hiding this comment.
Pull request overview
This PR fixes a concurrency race between configure and refresh-related operations by serializing shared locator reconfiguration with configuration publication, ensuring refreshes observe a coherent configuration boundary (Fixes #385).
Changes:
- Refactors
handle_configure()to apply configuration updates and reconfigure long-lived shared locators under the same configuration write lock. - Introduces
apply_configure_options()to centralize configure-state mutation + shared locator configuration. - Adds a regression test to ensure configuration readers cannot observe the new configuration state before shared locators have been fully reconfigured.
Show a summary per file
| File | Description |
|---|---|
crates/pet/src/jsonrpc.rs |
Serializes configuration publication with shared locator configuration and adds a regression test guarding against early visibility. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
There was a problem hiding this comment.
Pull request overview
This PR addresses Issue #385 by preventing configure and refresh from racing on shared locator configuration. It does this by ensuring locator reconfiguration and configuration-state publication occur under the same configuration write lock, so concurrent readers/refresh state sync can’t observe partially-applied configuration.
Changes:
- Refactors
configurehandling to apply config updates and reconfigure long-lived shared locators under a single configuration write lock (apply_configure_options). - Adds panic-guarding + rollback attempt for locator configuration failures to avoid publishing partially-applied configuration.
- Adds regression tests to verify configuration readers cannot observe the new configuration state before shared locators finish configuring.
Show a summary per file
| File | Description |
|---|---|
crates/pet/src/jsonrpc.rs |
Serializes configure publication with shared-locator reconfiguration; adds rollback-on-panic behavior and regression tests for the locking boundary. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
There was a problem hiding this comment.
Pull request overview
This PR addresses a correctness race in PET’s JSON-RPC server where configuration publication could interleave with shared locator reconfiguration / refresh-state sync, by serializing locator configuration with configuration state updates under the same configuration write lock.
Changes:
- Refactors
configurehandling to apply configuration updates and shared locator reconfiguration atomically via a newapply_configure_options()helper. - Adds defensive panic handling + rollback attempts for locator configuration/caching setup, and introduces regression tests to validate publication ordering and panic behavior.
- Adjusts a
pet-virtualenvwrappertest to canonicalize a temp project root path for stable assertions.
Show a summary per file
| File | Description |
|---|---|
crates/pet/src/jsonrpc.rs |
Serializes configure publication with shared locator reconfiguration; adds panic handling, rollback helper, and regression tests. |
crates/pet-virtualenvwrapper/src/environments.rs |
Canonicalizes the test project_root path to align with path normalization behavior. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 2
There was a problem hiding this comment.
Pull request overview
This PR addresses the race in #385 where configure could publish a new configuration generation while shared, long-lived locators were still being reconfigured (and while refresh-state sync could interleave with that reconfiguration). It does so by applying configuration updates and shared-locator reconfiguration under the same ConfigurationState write lock, and adds regression tests to enforce the intended visibility boundary.
Changes:
- Move
configure’s “publish new config generation” step behind a new helper that holds the configuration write lock while configuring shared locators. - Add rollback + panic handling so a panicking locator
configure()won’t publish partial configuration state. - Adjust a virtualenvwrapper test to canonicalize the project root path.
Show a summary per file
| File | Description |
|---|---|
crates/pet/src/jsonrpc.rs |
Serializes configure publication with shared locator configuration; adds rollback/panic safety and regression tests for the locking boundary. |
crates/pet-virtualenvwrapper/src/environments.rs |
Canonicalizes a test path to make .project path handling consistent. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
Summary:
Validation:
Fixes #385